Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix(router-cli): Fixes on Windows and Improvements #717

Merged
merged 4 commits into from
Sep 8, 2023

Conversation

Denny09310
Copy link
Contributor

This pull request addresses the following changes:

(fix) Fixed backslash on Windows:

(fix) Sanitized routePaths, import, and interface keys:

  • Improved code quality by sanitizing routePaths, imports, and interface keys, because on windows i've found cases on which the same problem as the backslashes occurs.

(enhanced) Made the parent search recursive:

  • Enhanced the parent route search functionality by making it recursive. This change improves the robustness of route identification and better aligns with project requirements.

(question) Layout Generation:

  • I have a question regarding the current layout generation method. During the fix of the code I've tried implementing an alternative approach that I believe could enhance the code's comprehensibility and maintainability. However, this new approach seems to disrupt the existing layout generation method. Before proceeding with any further changes, I would like to open a discussion about the best way to generate layouts, if interested. For now I'll leave here a simple explanation.

In the current project structure, we are using a pattern where there are separate files for layouts and index routes, leading to some complexities when organizing routes, especially when considering sub-routes within folders.

Current Project Structure:
posts.tsx is the layout for the route /posts and generates a key with /posts/.
posts.index.ts is the actual /posts route and generates the key /posts (without the last forward slash).
posts.$postId.tsx is the route that matches the postId property and generates the key /posts/$postId.
At least, this is what I had understood.

Challenges with the Current Approach:
Complexity when organizing routes, especially sub-routes within folders.
Odd generation behavior when attempting to place the last two files (posts.index.ts and posts.$postId.tsx) inside a folder called "posts". (I will retry getting this behavior asap with a brand new project).

Proposed Alternative Approach:
I propose an alternative approach to simplify route and layout generation:

Create a folder for the main route, e.g., posts. Inside the folder, have an index.tsx file, which would serve as both the layout for the main route and the actual /posts route. Also inside the folder, have a $postId.tsx file for routes that match the postId property. This file should generate the key /posts/$postId.

Benefits of the Alternative Approach:

  • Simplified organization: All related routes for a specific section are contained within a single folder, making it intuitive and organized.

  • Clear route generation: Each route file generates a key that directly corresponds to its URL path, eliminating confusion and redundancy.

I'm open to discussing this alternative approach with the team to gather insights and feedback.
I look forward to hearing your thoughts and suggestions on this proposed change.

Koja sig. Dennis added 4 commits September 5, 2023 17:26
(fix) Sanitized routePaths, import and interface keys
(enhanched) made the parent search recursive
(question) how layouts need to be generated?
(fix) Removed example, moved to another branch
@tannerlinsley
Copy link
Collaborator

Awesome work!

@tannerlinsley
Copy link
Collaborator

Let's discuss layout generation enhancement in Discord in a different PR

@tannerlinsley tannerlinsley merged commit 0b2cd04 into TanStack:beta Sep 8, 2023
@Denny09310
Copy link
Contributor Author

Denny09310 commented Sep 8, 2023

Great! I'll participate the discord group rn. Let me know where I can start the discussion

FugiTech pushed a commit to FugiTech/router that referenced this pull request Sep 12, 2023
* (fix) Fixed backslash on windows
(fix) Sanitized routePaths, import and interface keys
(enhanched) made the parent search recursive
(question) how layouts need to be generated?

* (feat) Added WIP example for file routing

* (fix) add debug json in ignored

* (fix) Added return type to 'hasParentRoute'
(fix) Removed example, moved to another branch

---------

Co-authored-by: Koja sig. Dennis <DKOJA@ceia-spa.com>
tannerlinsley pushed a commit that referenced this pull request Sep 12, 2023
* docs: fixed variable name error (#715)

used the wrong example in customStringifier example

* Fix(router-cli): Fixes on Windows and Improvements  (#717)

* (fix) Fixed backslash on windows
(fix) Sanitized routePaths, import and interface keys
(enhanched) made the parent search recursive
(question) how layouts need to be generated?

* (feat) Added WIP example for file routing

* (fix) add debug json in ignored

* (fix) Added return type to 'hasParentRoute'
(fix) Removed example, moved to another branch

---------

Co-authored-by: Koja sig. Dennis <DKOJA@ceia-spa.com>

* fix: types for useNavigate and useMatchRoute (#723)

* document passing Link isActive to children (#725)

---------

Co-authored-by: Frank Tran <ftransd@gmail.com>
Co-authored-by: Denny09310 <50493437+Denny09310@users.noreply.github.com>
Co-authored-by: Koja sig. Dennis <DKOJA@ceia-spa.com>
Co-authored-by: Blake Gentry <bgentry@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants